feat(HTTPReceiver): add invalidRequestSignatureHandler callback#2827
feat(HTTPReceiver): add invalidRequestSignatureHandler callback#2827mvanhorn wants to merge 2 commits intoslackapi:mainfrom
Conversation
mwbrooks
added
enhancement
|
Hey @mvanhorn 👋🏻 This sounds like a reasonable enhancement and thank you the explaining the motivation related to the I'll add a few maintainers to review this PR 👏🏻 In the meantime, can you please add tests for the new changes? I imagine that'll be one of the first requests from the reviewers as well. 🙇🏻 |
|
Thanks for the contribution! Unfortunately we can't verify the commit author(s): Matt Van Horn <m***@m***.local>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce Inc. Contributor License Agreement and this Pull Request will be revalidated. |
|
Added tests in b8d77a0 - three cases covering: custom handler receives correct args on signature failure, default noop handler doesn't throw, and missing headers pass undefined for signature/ts. Full suite passes (407 tests). |
|
@mvanhorn Super amazing changes more 🧪 ✨ The most recent commit author might've been configured strange - would it be possible to force push changes with an author that can sign the CLA? 🏛️ Rambles now but slackapi/node-slack-sdk#1135 becomes most curious 👾 |
Adds an optional invalidRequestSignatureHandler to HTTPReceiver, matching the callback added to AwsLambdaReceiver in PR slackapi#2154. When signature verification fails, the handler fires with the raw body, signature header, and timestamp. Defaults to a noop. Refs slackapi#2156
Cover three scenarios: custom handler called with correct args on signature failure, default noop handler doesn't throw, and missing headers pass undefined for signature/ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mvanhorn
force-pushed
the
osc/2156-invalid-sig-handler
branch
from
b8d77a0 to
4aa2f01
Compare
|
Fixed the commit author in 4aa2f01 — both commits now use my GitHub-linked noreply address. CLA should pass on recheck. All 407 tests still passing. Re: node-slack-sdk#1135 — interesting, hadn't seen that. The invalidRequestSignatureHandler pattern here could serve as a stepping stone toward that kind of unified request validation. |
3 participants
Adds invalidRequestSignatureHandler to HTTPReceiver.
Why this matters
PR #2154 added this callback to AwsLambdaReceiver. HTTPReceiver was missing it. Now HTTP apps can hook into signature verification failures too.
Changes
HTTPReceiverInvalidRequestSignatureHandlerArgsinterface. New option inHTTPReceiverOptions. Constructor wiring with noop default. Fires in the signature-verification catch block with rawBody, signature header, and timestamp.ExpressReceiver support can follow in a separate PR.
Testing
Codex ran
npm testagainst the change. All existing tests pass.Refs #2156
This contribution was developed with AI assistance (Claude Code).